avoid redundant prompt for http password in git-annex get that does autoinit
authorJoey Hess <joeyh@joeyh.name>
Fri, 9 Sep 2022 18:43:43 +0000 (14:43 -0400)
committerJoey Hess <joeyh@joeyh.name>
Fri, 9 Sep 2022 18:43:43 +0000 (14:43 -0400)
autoEnableSpecialRemotes runs a subprocess, and if the uuid for a git
remote has not been probed yet, that will do a http get that will prompt
for a password. And then the parent process will subsequently prompt
for a password when getting annexed files from the remote.

So the solution is for autoEnableSpecialRemotes to run remoteList before
the subprocess, which will probe for the uuid for the git remote in the
same process that will later be used to get annexed files.

But, Remote.Git imports Annex.Init, and Remote.List imports Remote.Git,
so Annex.Init cannot import Remote.List. Had to pass remoteList into
functions in Annex.Init to get around this dependency loop.

Annex/Init.hs
Command.hs
Command/Assistant.hs
Remote/Git.hs
doc/todo/not_ask_git_credentials_for_password_per_each_file.mdwn
doc/todo/not_ask_git_credentials_for_password_per_each_file/comment_4_4dca41df7506fa7aedf88d7618bbcf39._comment [new file with mode: 0644]

index c2dec9237b248e5d6b4d9232a0ebbbfad9e3a156..94f3ed5079641d583666fc95e1e73fa2f71e6cf6 100644 (file)
@@ -201,8 +201,8 @@ getInitializedVersion = do
  -
  - Checks repository version and handles upgrades too.
  -}
-ensureInitialized :: Annex ()
-ensureInitialized = getInitializedVersion >>= maybe needsinit checkUpgrade
+ensureInitialized :: Annex [Remote] -> Annex ()
+ensureInitialized remotelist = getInitializedVersion >>= maybe needsinit checkUpgrade
   where
        needsinit = ifM autoInitializeAllowed
                ( do
@@ -210,7 +210,7 @@ ensureInitialized = getInitializedVersion >>= maybe needsinit checkUpgrade
                                Right () -> noop
                                Left e -> giveup $ show e ++ "\n" ++
                                        "git-annex: automatic initialization failed due to above problems"
-                       autoEnableSpecialRemotes
+                       autoEnableSpecialRemotes remotelist
                , giveup "First run: git-annex init"
                )
 
@@ -254,13 +254,13 @@ guardSafeToUseRepo a = ifM (inRepo Git.Config.checkRepoConfigInaccessible)
  -
  - Checks repository version and handles upgrades too.
  -}
-autoInitialize :: Annex ()
-autoInitialize = getInitializedVersion >>= maybe needsinit checkUpgrade
+autoInitialize :: Annex [Remote] -> Annex ()
+autoInitialize remotelist = getInitializedVersion >>= maybe needsinit checkUpgrade
   where
        needsinit =
                whenM (initializeAllowed <&&> autoInitializeAllowed) $ do
                        initialize True Nothing Nothing
-                       autoEnableSpecialRemotes
+                       autoEnableSpecialRemotes remotelist
 
 {- Checks if a repository is initialized. Does not check version for ugrade. -}
 isInitialized :: Annex Bool
@@ -440,9 +440,17 @@ fixupUnusualReposAfterInit = do
 {- Try to enable any special remotes that are configured to do so.
  - 
  - The enabling is done in a child process to avoid it using stdio.
+ -
+ - The remotelist should be Remote.List.remoteList, which cannot
+ - be imported here due to a dependency loop.
  -}
-autoEnableSpecialRemotes :: Annex ()
-autoEnableSpecialRemotes = do
+autoEnableSpecialRemotes :: Annex [Remote] -> Annex ()
+autoEnableSpecialRemotes remotelist = do
+       -- Get all existing git remotes to probe for their uuid here,
+       -- so it is not done inside the child process. Doing it in there
+       -- could result in password prompts for http credentials,
+       -- which would then not end up cached in this process's state.
+       _ <- remotelist
        rp <- fromRawFilePath <$> fromRepo Git.repoPath
        withNullHandle $ \nullh -> gitAnnexChildProcess "init"
                [ Param "--autoenable" ]
index 21607e4962f7f0ce3fbde602b8f23a8f7f1c382c..09e7cb55be42fb4a2c8e075c464723c25b64ba0c 100644 (file)
@@ -28,6 +28,7 @@ import Utility.Daemon
 import Types.Transfer
 import Types.ActionItem
 import Types.WorkerPool as ReExported
+import Remote.List
 
 {- Generates a normal Command -}
 command :: String -> CommandSection -> String -> CmdParamsDesc -> (CmdParamsDesc -> CommandParser) -> Command
@@ -125,7 +126,7 @@ commonChecks :: [CommandCheck]
 commonChecks = [repoExists]
 
 repoExists :: CommandCheck
-repoExists = CommandCheck 0 ensureInitialized
+repoExists = CommandCheck 0 (ensureInitialized remoteList)
 
 notBareRepo :: Command -> Command
 notBareRepo = addCheck checkNotBareRepo
index f4dd9083cd4be4f529ad2b95fe4e87214ac5fbff..2a9bfc988766e609418d0ec1fde732591535c449 100644 (file)
@@ -16,6 +16,7 @@ import Config.Files.AutoStart
 import qualified BuildInfo
 import Utility.HumanTime
 import Assistant.Install
+import Remote.List
 
 import Control.Concurrent.Async
 
@@ -62,7 +63,7 @@ start o
                stop
        | otherwise = do
                liftIO ensureInstalled
-               ensureInitialized
+               ensureInitialized remoteList
                Command.Watch.start True (daemonOptions o) (startDelayOption o)
 
 startNoRepo :: AssistantOptions -> IO ()
index 48f2422cd85292982980ebc27df530e1ecec5dff..41ea016cf2b0c4947765db232cdad7703e1b87b5 100644 (file)
@@ -350,7 +350,7 @@ tryGitConfigRead autoinit r hasuuid
        readlocalannexconfig = do
                let check = do
                        Annex.BranchState.disableUpdate
-                       catchNonAsync autoInitialize $ \e ->
+                       catchNonAsync (autoInitialize (pure [])) $ \e ->
                                warning $ "remote " ++ Git.repoDescribe r ++
                                        ":"  ++ show e
                        Annex.getState Annex.repo
@@ -605,7 +605,7 @@ repairRemote r a = return $ do
        s <- Annex.new r
        Annex.eval s $ do
                Annex.BranchState.disableUpdate
-               ensureInitialized
+               ensureInitialized (pure [])
                a `finally` stopCoProcesses
 
 data LocalRemoteAnnex = LocalRemoteAnnex Git.Repo (MVar [(Annex.AnnexState, Annex.AnnexRead)])
@@ -647,7 +647,7 @@ onLocal' (LocalRemoteAnnex repo mv) a = liftIO (takeMVar mv) >>= \case
        [] -> do
                liftIO $ putMVar mv []
                v <- newLocal repo
-               go (v, ensureInitialized >> a)
+               go (v, ensureInitialized (pure []) >> a)
        (v:rest) -> do
                liftIO $ putMVar mv rest
                go (v, a)
index 27209b50989e90652b82da288aa43145b56e28bd..1848e92093e0e0ae7118621b343c58b0d6cbcde0 100644 (file)
@@ -31,3 +31,5 @@ and wondered if such excessive prompting could be avoided without engaging `git`
 
 [[!meta author=yoh]]
 [[!tag projects/repronim]]
+
+> [[fixed|done]] --[[Joey]]
diff --git a/doc/todo/not_ask_git_credentials_for_password_per_each_file/comment_4_4dca41df7506fa7aedf88d7618bbcf39._comment b/doc/todo/not_ask_git_credentials_for_password_per_each_file/comment_4_4dca41df7506fa7aedf88d7618bbcf39._comment
new file mode 100644 (file)
index 0000000..fc72aba
--- /dev/null
@@ -0,0 +1,19 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 4"""
+ date="2022-09-09T18:39:06Z"
+ content="""
+Fixed the autoinit case.
+
+I'm going to close this as done, despite the cases mentioned above where
+subprocesses might redundantly prompt for the credentials. 
+
+Another reason
+to not worry about those is that `git-annex sync` runs `git fetch` and `git
+push` (more than once), so there will be several password prompts there.
+So when git-annex runs a git-annex subprocess, it follows it's just as ok
+for it to do its own password prompts as it is for a git subprocess to do
+so. The solution to either is certianly to enable git's credential cache.
+So the scope of this todo has to be limited to prompting done by a single
+git-annex process.
+"""]]